Skip to content

Templates#1

Merged
MGatner merged 2 commits into
developfrom
templates
Nov 18, 2021
Merged

Templates#1
MGatner merged 2 commits into
developfrom
templates

Conversation

@MGatner

@MGatner MGatner commented Nov 17, 2021

Copy link
Copy Markdown
Member
  • Adds workflow templates
  • Adds config files examples for workflows
  • Updates docs with a lot more info

@MGatner

MGatner commented Nov 17, 2021

Copy link
Copy Markdown
Member Author

Hey team, please look over these well as I would like these to trickle down to our existing repos.

@samsonasik would you like to take a shot at a suitable Rector version?

@lonnieezell

Copy link
Copy Markdown
Member

All in all it looks fine, and I appreciate the updated readme!

My only question is - do we need Infection to run constantly? I get that it can be a good tool to use when updating your tests, but I feel it introduces a lot of extra noise when checking pull requests. As far as I'm concerned, as long as we make it easy to run manually, it doesn't need to be included on a PR.

Curious to see what others think, though.

@MGatner

MGatner commented Nov 17, 2021

Copy link
Copy Markdown
Member Author

@lonnieezell I picked up on that from our last Infection conversation 🤗

I've actually moved it to its own workflow, which is a change from what is live on Settings and Tasks. It means that the PHPUnit 8.0 tests run twice but it gives us the option of include/excluding the infection.yml workflow in places where we don't want it. I would definitely like to see it included in this repo, as most people have not been exposed to "test quality tools", but I'm fine removing it from existing libraries for now.

@lonnieezell

Copy link
Copy Markdown
Member

@MGatner I agree it has a place in this repo. And I'd love to be able to run it manually whenever. Just don't love it when looking over PRs, that's all. :)

And, to be fair, until you started using it here, I wasn't familiar with it either.

@MGatner

MGatner commented Nov 17, 2021

Copy link
Copy Markdown
Member Author

@lonnieezell No worries! Right now there is no template injection or anything, so files supplied here are for developers to copy in as they choose. Eventually I want to include a script to integrate these into repos, especially to merge updates, but for now I suggest that we publish these (once the team has had a chance to review) and then incorporate them manually into our current repos.

@samsonasik

Copy link
Copy Markdown
Member

@MGatner for rector, I suggest to use pinned version, eg: 0.12.4:
https://github.com/codeigniter4/CodeIgniter4/blob/d4db7b22dc8fb9901d94ab89c14f84fc662fff0e/composer.json#L27

and use dependabot to verify if it still working with next version.

@MGatner

MGatner commented Nov 17, 2021

Copy link
Copy Markdown
Member Author

Thanks @samsonasik. Would the repo's Config file be appropriate as a general configuration for other projects & libraries? Or is there a more generic version we can push?

This also brings up the point: should we delay publishing this repo until PHP 7.4 is the minimum and we can use more specific tooling?

@samsonasik

Copy link
Copy Markdown
Member

@MGatner I am not sure, if the "Config" you mean is the "rector.php" configuration, that possibly better to have per-project basis, like in current CodeIgniter4's repo:

https://github.com/codeigniter4/CodeIgniter4/blob/develop/rector.php

Comment thread README.md Outdated
Comment thread README.md Outdated
@MGatner

MGatner commented Nov 18, 2021

Copy link
Copy Markdown
Member Author

@samsonasik I will work on something and get you to review it.

@kenjis kenjis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see all the details of the workflow settings, but it looks generally good.

@MGatner

MGatner commented Nov 18, 2021

Copy link
Copy Markdown
Member Author

@kenjis since these are just recommendations and not usable directly it isn't so important that we get the workflows perfect. That said these are based on my own templates that I've been using and tweaking for many months, took this opportunity to clean them up and consolidate them.

I'm excited to be offering this officially alongside the framework, instead of it always be my own little corner.

@MGatner
MGatner merged commit 761d524 into develop Nov 18, 2021
@MGatner
MGatner deleted the templates branch November 22, 2021 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants